packages/device-client/src/lib/device.store.ts (2)

339-344: ⚠️ Potential issue | 🟡 Minor

JSDoc description inconsistency: "Get profile devices" should be "Delete profile device".

The @function delete annotation is correct, but the description on line 340 still says "Get profile devices" which doesn't match the delete operation. Similarly, the @param description on line 344 says "used to update a profile device" but should say "used to delete a profile device".

📝 Proposed fix for JSDoc description

       /**
-       * Get profile devices
+       * Delete a profile device
        *
        * `@async`
        * `@function` delete
-       * `@param` {ProfileDevicesQuery} query - The query used to update a profile device
+       * `@param` {ProfileDevicesQuery} query - The query used to delete a profile device
        * `@returns` {Promise<null | { error: unknown }>} - A promise that resolves to null or an error object if the response is not valid.
        */

---

316-321: ⚠️ Potential issue | 🟡 Minor

JSDoc description inconsistency: "Get profile devices" should describe the update operation.

The description on line 317 says "Get profile devices" but this is the update function. The description should reflect the update operation.

📝 Proposed fix for JSDoc description

       /**
-       * Get profile devices
+       * Update a profile device
        *
        * `@async`
        * `@function` update
        * `@param` {ProfileDevicesQuery} query - The query used to update a profile device

packages/device-client/src/lib/device.store.ts (1)

339-344: ⚠️ Potential issue | 🟡 Minor

Remaining copy-paste JSDoc issues in the profile section.

The @function tag fix on line 343 is correct, but the surrounding JSDoc still has leftover issues from copy-paste:

1. Line 340: Description says "Get profile devices" — should be "Delete a profile device" (same issue on line 317 for update, which says "Get profile devices" instead of "Update a profile device").
2. Line 344: @param says "The query used to update a profile device" — should say "delete".
3. Line 322 (the update method's @returns): Minor typo — "or or an error object" has a duplicated "or".

Since this PR targets comment fixes, consider cleaning these up as well.

📝 Proposed JSDoc fixes for the profile section

For the update method (lines 316–322):

      /**
-       * Get profile devices
+       * Update a profile device
        *
        * `@async`
        * `@function` update
        * `@param` {ProfileDevicesQuery} query - The query used to update a profile device
-       * `@returns` {Promise<ProfileDevice | { error: unknown }>} - A promise that resolves to the response data or or an error object if the response is not valid.
+       * `@returns` {Promise<ProfileDevice | { error: unknown }>} - A promise that resolves to the response data or an error object if the response is not valid.
        */

For the delete method (lines 339–344):

      /**
-       * Get profile devices
+       * Delete a profile device
        *
        * `@async`
        * `@function` delete
-       * `@param` {ProfileDevicesQuery} query - The query used to update a profile device
+       * `@param` {ProfileDevicesQuery} query - The query used to delete a profile device
        * `@returns` {Promise<null | { error: unknown }>} - A promise that resolves to null or an error object if the response is not valid.
        */

packages/sdk-effects/logger/README.md (1)

78-110: ⚠️ Potential issue | 🟡 Minor

Clarify that level filtering applies before delegating to custom logger.

The current text "all log calls will be delegated" is misleading—the implementation applies level filtering before delegation, so calls below the current level never reach the custom logger. Update to:

Suggested fix

-When a custom logger is provided, all log calls will be delegated to your implementation. If no custom logger is provided, the logger falls back to the browser's native `console` methods.
+When a custom logger is provided, log calls are first filtered by the current level, then delegated to your implementation. If no custom logger is provided, the logger falls back to the browser's native `console` methods.

packages/sdk-effects/iframe-manager/README.md (2)

49-54: ⚠️ Potential issue | 🔴 Critical

Critical: Inconsistent error handling documentation.

Lines 49-53 state that failures resolve with error objects, but line 54 says it returns "An Error" for validation failures (missing/empty successParams or errorParams). This is ambiguous:

• Does line 54 mean the promise rejects with an Error (throwing synchronously or rejecting the promise)?
• Or does it mean the promise resolves with an Error object?

This inconsistency will confuse developers about whether they need a try-catch block for validation errors.

Recommendation: Clarify whether validation errors reject the promise or resolve with an error object. If they reject, update the "On Failure" heading to distinguish between resolution-based errors and rejection-based errors.

---

89-103: ⚠️ Potential issue | 🟠 Major

Major: Catch block appears inconsistent with documented behavior.

The documentation states (lines 49-53) that internal errors resolve with objects like { type: 'internal_error', message: '...' }. However, the example's catch block (lines 89-103) attempts to handle these internal_error objects, which would never execute if they resolve rather than reject.

Additionally:

• Lines 94-98 check for error/error_description in the catch block, but these are already handled at lines 79-82 in the try block.
• If the promise always resolves (as stated in line 75), this entire catch block would only execute for the validation Error mentioned in line 54.

Recommendation: Clarify the catch block's purpose:

1. If internal_error cases truly resolve (per docs), remove lines 91-93 from the catch block
2. If error parameters truly resolve (per docs), remove lines 94-98 from the catch block
3. Keep the catch block only if line 54 validation errors actually reject (and document this clearly)

packages/device-client/src/lib/device.store.ts (1)

316-345: ⚠️ Potential issue | 🟡 Minor

JSDoc descriptions for profile.update and profile.delete still say "Get profile devices".

Lines 317 and 340 both read Get profile devices, but they document the update and delete methods respectively. The @function tags were correctly updated, but the summary lines were missed.

📝 Proposed fix

      /**
-       * Get profile devices
+       * Update a profile device
        *
        * `@async`
        * `@function` update

      /**
-       * Get profile devices
+       * Delete a profile device
        *
        * `@async`
        * `@function` delete

packages/sdk-effects/iframe-manager/README.md (2)

49-54: ⚠️ Potential issue | 🟡 Minor

API reference incorrectly states internal errors "resolve" — they actually reject.

Lines 51–53 describe timeout, cross-origin, and setup failures as resolving, but the implementation uses reject() for all three (lines 141, 156, and 165 in iframe-manager.effects.ts). Only the error-params case (line 50) truly resolves.

Suggested fix: split the section into "Resolves" (error params) and "Rejects" (internal errors).

Proposed doc fix

- **On Failure**: Resolves with:
-   - `ResolvedParams`: An object containing _all_ parsed query parameters if the final redirect URL contains any key listed in `errorParams`.
-   - An object `{ type: 'internal_error', message: 'iframe timed out' }` if the specified `timeout` is reached before a success or error condition is met.
-   - An object `{ type: 'internal_error', message: 'unexpected failure' }` if there's an error accessing the iframe's content window (most likely due to a cross-origin redirect that wasn't expected or handled).
-   - An object `{ type: 'internal_error', message: 'error setting up iframe' }` if there was an issue creating or configuring the iframe initially.
-   - An `Error` if `successParams` or `errorParams` are missing or empty during setup.
+ **On Error Params**: Resolves with `ResolvedParams` containing _all_ parsed query parameters if the final redirect URL contains any key listed in `errorParams`. The caller must inspect the result for error keys.
+ **On Failure (Rejects)**: Rejects with:
+   - `{ type: 'internal_error', message: 'iframe timed out' }` if the specified `timeout` is reached.
+   - `{ type: 'internal_error', message: 'unexpected failure' }` on cross-origin access errors.
+   - `{ type: 'internal_error', message: 'error setting up iframe' }` on iframe creation failure.
+   - Throws an `Error` synchronously if `successParams` or `errorParams` are missing or empty.

---

89-102: ⚠️ Potential issue | 🟡 Minor

Catch block in example contains dead code for the new error-params flow.

Lines 94–98 check for errorResult.error / errorResult.error_description inside the catch block, but with the new behavior, error params now resolve (handled on lines 79–82 in the try block). This branch is unreachable and will confuse readers about the new contract.

Suggested cleanup

   } catch (errorResult) {
-    // Failure case: Check if it's a known error from the server or an internal error
     if (errorResult && errorResult.type === 'internal_error') {
       // Timeout or iframe access error
       console.error(`Iframe operation failed: ${errorResult.message}`);
-    } else if (errorResult && (errorResult.error || errorResult.error_description)) {
-      // Error reported by the authorization server via errorParams
-      console.error('Silent login failed. Server returned error:', errorResult);
-      // const errorCode = errorResult.error;
-      // const errorDesc = errorResult.error_description;
     } else {
       // Other unexpected error
       console.error('An unexpected error occurred:', errorResult);
     }
   }

packages/sdk-effects/sdk-request-middleware/README.md (1)

70-75: ⚠️ Potential issue | 🟡 Minor

Fix switch-case syntax in snippet.

case 'DAVINCI_NEXT' is missing a colon, which makes the example invalid for copy/paste.

🛠️ Proposed fix

-      case 'DAVINCI_NEXT'
+      case 'DAVINCI_NEXT':

packages/sdk-effects/iframe-manager/README.md (1)

89-103: ⚠️ Potential issue | 🟡 Minor

Stale error-handling branch in the catch block of the usage example.

Lines 94-98 check for errorResult.error / errorResult.error_description in the catch block, but with the new resolve-on-error-params behavior, server-reported errors will never reach catch — they'll be handled by the success path at lines 79-82. This dead branch in the example will confuse consumers trying to adopt the new API.

Suggested cleanup

   } catch (errorResult) {
-    // Failure case: Check if it's a known error from the server or an internal error
     if (errorResult && errorResult.type === 'internal_error') {
       // Timeout or iframe access error
       console.error(`Iframe operation failed: ${errorResult.message}`);
-    } else if (errorResult && (errorResult.error || errorResult.error_description)) {
-      // Error reported by the authorization server via errorParams
-      console.error('Silent login failed. Server returned error:', errorResult);
-      // const errorCode = errorResult.error;
-      // const errorDesc = errorResult.error_description;
     } else {
       // Other unexpected error
       console.error('An unexpected error occurred:', errorResult);
     }
   }